Feature/rdkemw 12692 by Koky2701 · Pull Request #438 · rdkcentral/rialto

Koky2701 · 2026-01-28T11:22:24Z

Summary: 'report_decode_errors' property missing from RialtoMSEVideoSink
Type: Feature
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-12692

Summary: 'report_decode_errors' property missing from RialtoMSEVideoSink Type: Feature Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

…linemodule.proto Summary: 'report_decode_errors' property missing from RialtoMSEVideoSink Type: Feature Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

Summary: 'report_decode_errors' property missing from RialtoMSEVideoSink Type: Feature Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

github-actions · 2026-01-28T11:22:34Z

Pull request must be merged with a description containing the required fields,

Summary:
Type: Feature/Fix/Cleanup
Test Plan:
Jira:

If there is no jira releated to this change, please put 'Jira: NO-JIRA'.

Description can be changed by editing the top comment on your pull request and making a new commit.

github-actions · 2026-01-28T11:22:36Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

Copilot

Pull request overview

This pull request adds support for the 'report_decode_errors' property for RialtoMSEVideoSink by implementing a complete feature path from client to GStreamer player. The feature allows enabling or disabling decode error reporting for video sources.

Changes:

Added setReportDecodeErrors API across all layers (client IPC, server service, and GStreamer player)
Implemented SetReportDecodeErrors task in the GStreamer player task system
Added protocol buffer definitions for setReportDecodeErrors RPC and GetQueuedFrames RPC (the latter appears unimplemented)

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
proto/mediapipelinemodule.proto	Added ReportDecodeErrors and GetQueuedFrames message definitions and RPC methods
media/public/include/IMediaPipeline.h	Added setReportDecodeErrors interface method
media/client/main/include/MediaPipeline.h	Added setReportDecodeErrors method declaration
media/client/main/include/MediaPipelineProxy.h	Added setReportDecodeErrors proxy method
media/client/main/source/MediaPipeline.cpp	Implemented client-side setReportDecodeErrors
media/client/ipc/interface/IMediaPipelineIpc.h	Added IPC interface for setReportDecodeErrors
media/client/ipc/include/MediaPipelineIpc.h	Added IPC implementation method declaration
media/client/ipc/source/MediaPipelineIpc.cpp	Implemented IPC client for setReportDecodeErrors
media/server/ipc/include/MediaPipelineModuleService.h	Added server IPC method declaration
media/server/ipc/source/MediaPipelineModuleService.cpp	Implemented server IPC handler
media/server/service/include/IMediaPipelineService.h	Added service interface method
media/server/service/source/MediaPipelineService.h	Added service method declaration
media/server/service/source/MediaPipelineService.cpp	Implemented service-layer routing
media/server/main/include/MediaPipelineServerInternal.h	Added server internal method declarations
media/server/main/source/MediaPipelineServerInternal.cpp	Implemented main thread coordination
media/server/gstplayer/interface/IGstGenericPlayer.h	Added GStreamer player interface method
media/server/gstplayer/include/IGstGenericPlayerPrivate.h	Added private interface for worker thread
media/server/gstplayer/include/GstGenericPlayer.h	Added method declarations
media/server/gstplayer/source/GstGenericPlayer.cpp	Implemented GStreamer player methods
media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h	Added task factory interface for SetReportDecodeErrors
media/server/gstplayer/include/tasks/generic/GenericPlayerTaskFactory.h	Added task factory method declaration
media/server/gstplayer/source/tasks/generic/GenericPlayerTaskFactory.cpp	Implemented task factory method
media/server/gstplayer/include/tasks/generic/SetReportDecodeErrors.h	New task class header
media/server/gstplayer/source/tasks/generic/SetReportDecodeErrors.cpp	New task class implementation
media/server/gstplayer/CMakeLists.txt	Added SetReportDecodeErrors.cpp to build

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-28T11:27:38Z

media/public/include/IMediaPipeline.h

+     * This method is asynchronous, it will set the "Report Decode Errors" property
+     *
+     * @param[in] sourceId  : The source id. Value should be set to the MediaSource.id returned after attachSource()
+     * @param[in] immediateOutput : Set Report Decode Errors mode on the sink


The parameter name in the documentation is incorrect. It should be "reportDecodeErrors" instead of "immediateOutput" to match the actual parameter name.

Suggested change

* @param[in] immediateOutput : Set Report Decode Errors mode on the sink

* @param[in] reportDecodeErrors : Set Report Decode Errors mode on the sink

Copilot · 2026-01-28T11:27:38Z

media/client/ipc/interface/IMediaPipelineIpc.h

+     * This method is asynchronous, it will set the "Report Decode Errors" property
+     *
+     * @param[in] sourceId  : The source id. Value should be set to the MediaSource.id returned after attachSource()
+     * @param[in] immediateOutput : Set Report Decode Errors mode on the sink


The parameter name in the documentation is incorrect. It should be "reportDecodeErrors" instead of "immediateOutput" to match the actual parameter name.

Suggested change

* @param[in] immediateOutput : Set Report Decode Errors mode on the sink

* @param[in] reportDecodeErrors : Set Report Decode Errors mode on the sink

Copilot · 2026-01-28T11:27:39Z

media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h

+     * @brief Creates a setReportDecodeErrors task.
+     *
+     * @param[in] context         : The GstPlayer context
+     * @param[in] player          : The GstPlayer instance
+     * @param[in] type            : The media source type
+     * @param[in] immediateOutput : the value to set for report decode error
+     *
+     * @retval the new ProcessAudioGap task instance.


The return value documentation is incorrect. It should describe what task instance is being returned (SetReportDecodeErrors task) rather than stating "the new ProcessAudioGap task instance". This appears to be copy-pasted from another method without being updated.

Suggested change

* @brief Creates a setReportDecodeErrors task.

*

* @param[in] context : The GstPlayer context

* @param[in] player : The GstPlayer instance

* @param[in] type : The media source type

* @param[in] immediateOutput : the value to set for report decode error

*

* @retval the new ProcessAudioGap task instance.

* @brief Creates a SetReportDecodeErrors task.

*

* @param[in] context : The GstPlayer context

* @param[in] player : The GstPlayer instance

* @param[in] type : The media source type

* @param[in] reportDecodeErrors: the value to set for report decode errors

*

* @retval the new SetReportDecodeErrors task instance.

Copilot · 2026-01-28T11:27:39Z

media/server/gstplayer/include/tasks/generic/SetReportDecodeErrors.h

+#ifndef FIREBOLT_RIALTO_SERVER_TASKS_GENERIC_SET_REPORT_DECODE_ERROR_H_
+#define FIREBOLT_RIALTO_SERVER_TASKS_GENERIC_SET_REPORT_DECODE_ERROR_H_


The header guard name is inconsistent. It uses "SET_REPORT_DECODE_ERROR_H_" (singular) but should be "SET_REPORT_DECODE_ERRORS_H_" (plural) to match the class name SetReportDecodeErrors and follow the naming convention seen in other files.

Copilot · 2026-01-28T11:27:39Z

media/server/gstplayer/include/tasks/generic/SetReportDecodeErrors.h

+};
+} // namespace firebolt::rialto::server::tasks::generic
+
+#endif // FIREBOLT_RIALTO_SERVER_TASKS_GENERIC_SET_REPORT_DECODE_ERROR_H_


The header guard name is inconsistent. It uses "SET_REPORT_DECODE_ERROR_H_" (singular) but should be "SET_REPORT_DECODE_ERRORS_H_" (plural) to match the class name SetReportDecodeErrors and follow the naming convention seen in other files.

Copilot · 2026-01-28T11:27:40Z

proto/mediapipelinemodule.proto

+/**
+ * @fn void getQueuedFrames(int session_id, int source_id, uint32_t &queued_frames)
+ * @brief Gets the "Queued Frames" property for this source.
+ *
+ * @param[in]  session_id        The id of the A/V session.
+ * @param[in]  source_id         The id of the media source.
+ * @param[out] queued_frames  Get queued frames on the decoder
+ *
+ */
+message GetQueuedFramesRequest {
+    optional int32 session_id = 1 [default = -1];
+    optional int32 source_id = 2 [default = -1];
+}
+message GetQueuedFramesResponse {
+    optional uint32 queued_frames = 1 [default = -1];
+}


The GetQueuedFramesRequest and GetQueuedFramesResponse message definitions are added but there is no corresponding implementation in the server IPC layer, service layer, or client. This appears to be an incomplete feature addition. Either the implementation should be provided or these proto definitions should be removed.

Copilot · 2026-01-28T11:27:40Z

media/server/gstplayer/source/tasks/generic/SetReportDecodeErrors.cpp

+    if (m_type != MediaSourceType::VIDEO)
+    {
+        RIALTO_SERVER_LOG_ERROR("SetReportDecodeErrors not currently supported for non-video");
+    }
+
+    if (m_context.pipeline)
+    {
+        m_player.setReportDecodeErrors(m_reportDecodeErrors);
+    }


The function logs an error when the media source type is not VIDEO but continues execution and calls m_player.setReportDecodeErrors anyway. This appears to be a logic error. If the feature is not supported for non-video sources, the function should return early after logging the error, similar to how validation is typically handled.

Copilot · 2026-01-28T11:27:40Z

media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h

+     * @param[in] context         : The GstPlayer context
+     * @param[in] player          : The GstPlayer instance
+     * @param[in] type            : The media source type
+     * @param[in] immediateOutput : the value to set for report decode error


The parameter name in the documentation is incorrect. It should be "reportDecodeErrors" instead of "immediateOutput" to match the actual parameter name.

Suggested change

* @param[in] context : The GstPlayer context

* @param[in] player : The GstPlayer instance

* @param[in] type : The media source type

* @param[in] immediateOutput : the value to set for report decode error

* @param[in] context : The GstPlayer context

* @param[in] player : The GstPlayer instance

* @param[in] type : The media source type

* @param[in] reportDecodeErrors: the value to set for report decode errors

Copilot · 2026-01-28T11:27:41Z

media/server/main/include/MediaPipelineServerInternal.h

+     * This method is asynchronous
+     *
+     * @param[in] sourceId : The source id. Value should be set to the MediaSource.id returned after attachSource()
+     * @param[in] reportDecodeErrors : The desired immediate output mode on the sink


The parameter description is incorrect. It states "The desired immediate output mode on the sink" but should state something like "The desired report decode errors mode on the sink" or similar to accurately describe the reportDecodeErrors parameter.

Suggested change

* @param[in] reportDecodeErrors : The desired immediate output mode on the sink

* @param[in] reportDecodeErrors : The desired report decode errors mode on the sink

Copilot · 2026-01-28T11:27:41Z

media/server/gstplayer/source/GstGenericPlayer.cpp

+    }
+    else
+    {
+        RIALTO_SERVER_LOG_DEBUG("Pending an report_decode_errors, decoder is NULL");


Grammar error in the log message. It should be "Pending a report_decode_errors" instead of "Pending an report_decode_errors" since "report" starts with a consonant sound.

Suggested change

RIALTO_SERVER_LOG_DEBUG("Pending an report_decode_errors, decoder is NULL");

RIALTO_SERVER_LOG_DEBUG("Pending a report_decode_errors, decoder is NULL");

Copilot

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 25 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-03T14:24:47Z

media/server/service/source/MediaPipelineService.cpp

+bool MediaPipelineService::setReportDecodeErrors(int sessionId, int32_t sourceId, bool reportDecodeErrors)
+{
+    RIALTO_SERVER_LOG_INFO("MediaPipelineService requested to setReportDecodeErrors, session id: %d", sessionId);
+
+    std::lock_guard<std::mutex> lock{m_mediaPipelineMutex};
+    auto mediaPipelineIter = m_mediaPipelines.find(sessionId);
+    if (mediaPipelineIter == m_mediaPipelines.end())
+    {
+        RIALTO_SERVER_LOG_ERROR("Session with id: %d does not exists", sessionId);
+        return false;
+    }
+    return mediaPipelineIter->second->setReportDecodeErrors(sourceId, reportDecodeErrors);
+}


The new setReportDecodeErrors method is missing unit test coverage. Tests should be added similar to the existing setImmediateOutput tests to verify both success and failure cases.

Copilot · 2026-02-03T14:24:47Z

media/server/service/source/MediaPipelineService.cpp

+bool MediaPipelineService::getQueuedFrames(int sessionId, int32_t sourceId, uint32_t &queuedFrames)
+{
+    RIALTO_SERVER_LOG_INFO("MediaPipelineService requested to getQueuedFrames, session id: %d", sessionId);
+
+    std::lock_guard<std::mutex> lock{m_mediaPipelineMutex};
+    auto mediaPipelineIter = m_mediaPipelines.find(sessionId);
+    if (mediaPipelineIter == m_mediaPipelines.end())
+    {
+        RIALTO_SERVER_LOG_ERROR("Session with id: %d does not exists", sessionId);
+        return false;
+    }
+    return mediaPipelineIter->second->getQueuedFrames(sourceId, queuedFrames);
+}


The new getQueuedFrames method is missing unit test coverage. Tests should be added to verify both success and failure cases.

Copilot · 2026-02-03T14:24:47Z

media/server/main/source/MediaPipelineServerInternal.cpp

+bool MediaPipelineServerInternal::setReportDecodeErrors(int32_t sourceId, bool reportDecodeErrors)
+{
+    RIALTO_SERVER_LOG_DEBUG("entry:");
+
+    bool result;
+    auto task = [&]() { result = setReportDecodeErrorsInternal(sourceId, reportDecodeErrors); };
+
+    m_mainThread->enqueueTaskAndWait(m_mainThreadClientId, task);
+    return result;
+}
+
+bool MediaPipelineServerInternal::setReportDecodeErrorsInternal(int32_t sourceId, bool reportDecodeErrors)
+{
+    if (!m_gstPlayer)
+    {
+        RIALTO_SERVER_LOG_ERROR("Failed - Gstreamer player has not been loaded");
+        return false;
+    }
+    auto sourceIter = std::find_if(m_attachedSources.begin(), m_attachedSources.end(),
+                                   [sourceId](const auto &src) { return src.second == sourceId; });
+    if (sourceIter == m_attachedSources.end())
+    {
+        RIALTO_SERVER_LOG_ERROR("Failed - Source not found");
+        return false;
+    }
+
+    return m_gstPlayer->setReportDecodeErrors(sourceIter->first, reportDecodeErrors);
+}


The new setReportDecodeErrors and setReportDecodeErrorsInternal methods are missing unit test coverage. Tests should be added to verify the threading model, source validation, and error handling.

Copilot · 2026-02-03T14:24:47Z

media/client/ipc/interface/IMediaPipelineIpc.h

+     *
+     * This method is sychronous, it gets the queued frames property
+     *
+     * @param[in] sourceId  : The source id. Value should be get to the MediaSource.id returned after attachSource()


Spelling error in parameter description: "get" should be "set" (copy-paste error from the getter above).

Suggested change

* @param[in] sourceId : The source id. Value should be get to the MediaSource.id returned after attachSource()

* @param[in] sourceId : The source id. Value should be set to the MediaSource.id returned after attachSource()

Copilot · 2026-02-03T14:24:48Z

media/client/ipc/source/MediaPipelineIpc.cpp

+    // check the result
+    if (ipcController->Failed())
+    {
+        RIALTO_CLIENT_LOG_ERROR("failed to get immediate-output due to '%s'", ipcController->ErrorText().c_str());


Error message is incorrect: This function gets queued frames, not immediate-output. The error message should be "failed to get queued frames due to '%s'".

Suggested change

RIALTO_CLIENT_LOG_ERROR("failed to get immediate-output due to '%s'", ipcController->ErrorText().c_str());

RIALTO_CLIENT_LOG_ERROR("failed to get queued frames due to '%s'", ipcController->ErrorText().c_str());

Copilot · 2026-02-03T14:24:52Z

media/server/main/source/MediaPipelineServerInternal.cpp

+bool MediaPipelineServerInternal::getQueuedFrames(int32_t sourceId, uint32_t &queuedFrames)
+{
+    RIALTO_SERVER_LOG_DEBUG("entry:");
+
+    bool result;
+    auto task = [&]() { result = getQueuedFramesInternal(sourceId, queuedFrames); };
+
+    m_mainThread->enqueueTaskAndWait(m_mainThreadClientId, task);
+    return result;
+}
+
+bool MediaPipelineServerInternal::getQueuedFramesInternal(int32_t sourceId, uint32_t &queuedFrames)
+{
+    if (!m_gstPlayer)
+    {
+        RIALTO_SERVER_LOG_ERROR("Failed - Gstreamer player has not been loaded");
+        return false;
+    }
+    auto sourceIter = std::find_if(m_attachedSources.begin(), m_attachedSources.end(),
+                                   [sourceId](const auto &src) { return src.second == sourceId; });
+    if (sourceIter == m_attachedSources.end())
+    {
+        RIALTO_SERVER_LOG_ERROR("Failed - Source not found");
+        return false;
+    }
+    return m_gstPlayer->getQueuedFrames(sourceIter->first, queuedFrames);
+}


The new getQueuedFrames and getQueuedFramesInternal methods are missing unit test coverage. Tests should be added to verify the threading model, source validation, and error handling.

Copilot · 2026-02-03T14:24:53Z

media/client/main/source/MediaPipeline.cpp

+bool MediaPipeline::setReportDecodeErrors(int32_t sourceId, bool reportDecodeErrors)
+{
+    return m_mediaPipelineIpc->setReportDecodeErrors(sourceId, reportDecodeErrors);
+}
+
+bool MediaPipeline::getQueuedFrames(int32_t sourceId, uint32_t &queuedFrames)
+{
+    return m_mediaPipelineIpc->getQueuedFrames(sourceId, queuedFrames);
+}


The new setReportDecodeErrors and getQueuedFrames methods are missing unit test coverage. Tests should be added to verify the forwarding to the IPC layer.

Copilot · 2026-02-03T14:24:53Z

media/server/gstplayer/interface/IGstGenericPlayer.h

+    /**
+     * @brief Sets the "Report Decode Error" property for this source.
+     *
+     * @param[in] mediaSourceType : The media source type
+     * @param[in] reportDecodeErrors : Set report decode error
+     *
+     * @retval true on success.
+     */
+    virtual bool setReportDecodeErrors(const MediaSourceType &mediaSourceType, bool reportDecodeErrors) = 0;
+
+    /**
+     * @brief Gets the queued frames for this source.
+     *
+     * @param[in] mediaSourceType : The media source type
+     * @param[out] queuedFrames : Get queued frames mode on the decoder
+     *
+     * @retval true on success.
+     */
+    virtual bool getQueuedFrames(const MediaSourceType &mediaSourceType, uint32_t &queuedFrames) = 0;
+


The GstGenericPlayerMock is missing MOCK_METHOD declarations for the new setReportDecodeErrors and getQueuedFrames methods. These need to be added to maintain consistency with the interface and enable proper unit testing.

Copilot · 2026-02-03T14:24:53Z

media/server/main/include/MediaPipelineServerInternal.h

+    bool setReportDecodeErrors(int32_t sourceId, bool reportDecodeErrors) override;
+
+    bool getQueuedFrames(int32_t sourceId, uint32_t &queuedFrames) override;
+


The MediaPipelineServerInternalMock is missing MOCK_METHOD declarations for the new setReportDecodeErrors and getQueuedFrames methods. These need to be added after the setImmediateOutput/getImmediateOutput methods to maintain consistency with the interface and enable proper unit testing.

Copilot · 2026-02-03T14:24:53Z

media/server/gstplayer/source/GstGenericPlayer.cpp

+bool GstGenericPlayer::setReportDecodeErrors(bool reportDecodeErrors)
+{
+    bool result{false};
+    GstElement *decoder = getDecoder(MediaSourceType::VIDEO);


The implementation hardcodes MediaSourceType::VIDEO, but the public API accepts a mediaSourceType parameter. This creates an inconsistency where the caller can specify any source type, but it will always be applied to the VIDEO decoder. Either the implementation should respect the mediaSourceType parameter, or the API should not accept it at all.

Suggested change

bool GstGenericPlayer::setReportDecodeErrors(bool reportDecodeErrors)

{

bool result{false};

GstElement *decoder = getDecoder(MediaSourceType::VIDEO);

bool GstGenericPlayer::setReportDecodeErrors(MediaSourceType mediaSourceType, bool reportDecodeErrors)

{

bool result{false};

GstElement *decoder = getDecoder(mediaSourceType);

Summary: 'queued_buffers' property missing from RialtoMSEVideoSink Type: Feature Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

Koky2701 · 2026-02-10T17:00:25Z

I have read the CLA Document and I hereby sign the CLA

github-actions · 2026-02-10T17:07:49Z

tests/componenttests/server/tests/mediaPipeline/PipelinePropertyTest.cpp:350:0: style: The function 'setReportDecodeErrorsFailure' is never used. [unusedFunction]
void setReportDecodeErrorsFailure()
^
tests/componenttests/server/tests/mediaPipeline/PipelinePropertyTest.cpp:359:0: style: The function 'getQueuedFramesFailure' is never used. [unusedFunction]
void getQueuedFramesFailure()
^
nofile:0:0: information: Active checkers: 161/592 (use --checkers-report= to see details) [checkersReport]

rdkcmf-jenkins · 2026-02-10T17:11:20Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 5 files pending identification.

Protex Server Path: /home/blackduck/github/rialto/438/rdkcentral/rialto
Commit: 1fe58a5

Report detail: gist'

Copilot

Pull request overview

Copilot reviewed 62 out of 62 changed files in this pull request and generated 17 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-10T17:13:41Z

tests/componenttests/server/common/ActionTraits.h

+struct SetReportDecodeErrors
+{
+    using RequestType = ::firebolt::rialto::SetReportDecodeErrorsRequest;
+    using ResponseType = ::firebolt::rialto::SetReportDecodeErrorsResponse;
+    using Stub = ::firebolt::rialto::MediaPipelineModule_Stub;
+    static constexpr auto m_kFunction{&Stub::setReportDecodeErrors};
+};
+
+struct GetQueuedFrames
+{
+    using RequestType = ::firebolt::rialto::GetQueuedFramesRequest;
+    using ResponseType = ::firebolt::rialto::GetQueuedFramesResponse;
+    using Stub = ::firebolt::rialto::MediaPipelineModule_Stub;
+    static constexpr auto m_kFunction{&Stub::getQueuedFrames};
+};


ActionTraits::SetReportDecodeErrors uses SetReportDecodeErrorsRequest/Response, but the proto currently defines ReportDecodeErrorsRequest/Response. Align these types (either by renaming the proto messages or updating the trait).

Copilot · 2026-02-10T17:13:42Z

...tests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTestsFixture.cpp

+constexpr bool kReportDecodeErrorsVal{false};
+constexpr bool kQueuedFramesVal{123};


kQueuedFramesVal is declared as bool but used as a queued-frame count (set to 123 and written into a uint32_t &). This truncates the test value and can mask issues; it should be a uint32_t constant.

Copilot · 2026-02-10T17:13:42Z

...tests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTestsFixture.cpp

+{
+    firebolt::rialto::SetReportDecodeErrorsRequest request;
+    firebolt::rialto::SetReportDecodeErrorsResponse response;
+
+    request.set_session_id(kHardcodedSessionId);
+    request.set_immediate_output(kReportDecodeErrorsVal);
+
+    m_service->setReportDecodeErrors(m_controllerMock.get(), &request, &response, m_closureMock.get());


These helpers use SetReportDecodeErrorsRequest/Response and set an immediate_output field, but the proto defines ReportDecodeErrorsRequest with a report_decode_errors field. As written, this won't compile and also won't exercise the correct request field; update the request/response types and set report_decode_errors (and source_id if the other tests do).

Copilot · 2026-02-10T17:13:42Z

tests/unittests/media/server/main/mediaPipeline/MiscellaneousFunctionsTest.cpp

+    uint32_t queuedFrames;
+    mainThreadWillEnqueueTaskAndWait();
+    EXPECT_CALL(*m_gstPlayerMock, getQueuedFrames(_, _)).WillOnce(DoAll(SetArgReferee<1>(123), Return(true)));
+    EXPECT_TRUE(m_mediaPipeline->getQueuedFrames(videoSourceId, queuedFrames));
+    EXPECT_EQ(queuedFrames, true);
+
+    mainThreadWillEnqueueTaskAndWait();
+    EXPECT_CALL(*m_gstPlayerMock, getQueuedFrames(_, _)).WillOnce(DoAll(SetArgReferee<1>(123), Return(true)));
+    EXPECT_TRUE(m_mediaPipeline->getQueuedFrames(videoSourceId, queuedFrames));
+    EXPECT_EQ(queuedFrames, false);
+}


The test sets queuedFrames to 123 via the mock, but then compares it to true/false. This will fail (123 != true) and doesn't validate the correct behavior. Compare against the expected numeric values (and set different values for the two calls if you want to test both cases).

Copilot · 2026-02-10T17:13:42Z

proto/mediapipelinemodule.proto

+message ReportDecodeErrorsRequest {
+    optional int32 session_id = 1 [default = -1];
+    optional int32 source_id  = 2 [default = -1];
+    optional bool report_decode_errors = 3 [default = false];
+}
+
+message ReportDecodeErrorsResponse {


The newly added proto messages are named ReportDecodeErrorsRequest/Response, but the rpc is setReportDecodeErrors and the rest of this proto uses the SetXxxRequest/Response naming convention (e.g., SetImmediateOutputRequest). This mismatch is already reflected in the tests/helpers referencing SetReportDecodeErrorsRequest, causing compile failures; rename the messages (or update all call sites) so the API is consistent end-to-end.

Suggested change

message ReportDecodeErrorsRequest {

optional int32 session_id = 1 [default = -1];

optional int32 source_id = 2 [default = -1];

optional bool report_decode_errors = 3 [default = false];

}

message ReportDecodeErrorsResponse {

message SetReportDecodeErrorsRequest {

optional int32 session_id = 1 [default = -1];

optional int32 source_id = 2 [default = -1];

optional bool report_decode_errors = 3 [default = false];

}

message SetReportDecodeErrorsResponse {

Copilot · 2026-02-10T17:13:45Z

media/server/gstplayer/source/tasks/generic/SetReportDecodeErrors.cpp

+    if (m_context.pipeline)
+    {
+        RIALTO_SERVER_LOG_DEBUG("Pipeline not available yet - cannot apply report_decode_errors setting");
+        return;
+    }


The pipeline check is inverted: if (m_context.pipeline) currently logs 'Pipeline not available yet' and returns, which prevents applying the setting when the pipeline actually exists. This should only early-return when the pipeline is not available, and should call m_player.setReportDecodeErrors(...) when it is.

Copilot · 2026-02-10T17:13:45Z

tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp

+void GenericTasksTestsBase::shouldSetReportDecodeErrors()
+{
+    EXPECT_CALL(testContext->m_gstPlayer, setReportDecodeErrors()).WillOnce(Return(true));
+}


shouldSetReportDecodeErrors() sets an expectation on setReportDecodeErrors() with no arguments, but the mocked method takes a bool reportDecodeErrors parameter. This will not compile; update the expectation to include the expected bool value (and align with the task trigger).

Copilot · 2026-02-10T17:13:45Z

tests/unittests/media/server/service/mediaPipelineService/MediaPipelineServiceTestsFixture.cpp

+void MediaPipelineServiceTests::getQueuedFramesShouldSucceed()
+{
+    uint32_t queuedFr;
+    EXPECT_TRUE(m_sut->getQueuedFrames(kSessionId, kSourceId, queuedFr));
+    EXPECT_TRUE(immOp);
+}


getQueuedFramesShouldSucceed() references immOp, which is not declared in this function (likely a copy/paste from getImmediateOutputShouldSucceed). This will not compile and also doesn't validate the returned queued frame count; assert on queuedFr (e.g., expected 123) instead.

Copilot · 2026-02-10T17:13:45Z

tests/componenttests/server/common/MessageBuilders.h

+::firebolt::rialto::SetReportDecodeErrorsRequest createSetReportDecodeErrorsRequest(int sessionId, int sourceId,
+                                                                              bool reportDecodeErrors);
+::firebolt::rialto::GetQueuedFramesRequest createGetQueuedFramesRequest(int sessionId, int sourceId);


This header declares/returns ::firebolt::rialto::SetReportDecodeErrorsRequest, but the proto defines ReportDecodeErrorsRequest (and the service signature uses ReportDecodeErrorsRequest). This mismatch will break compilation; update the builder to use the actual generated proto type (or rename the proto message to match the builder).

Copilot · 2026-02-10T17:13:46Z

tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp

+TEST_F(GstGenericPlayerPrivateTest, shouldFailToSetReportDecodeErrorsIfSinkIsNull)
+{
+    EXPECT_CALL(*m_glibWrapperMock, gObjectGetStub(_, StrEq(kVideoSinkStr), _))
+        .WillOnce(Invoke(
+            [&](gpointer object, const gchar *first_property_name, void *element)
+            {
+                GstElement **elementPtr = reinterpret_cast<GstElement **>(element);
+                *elementPtr = nullptr;
+            }));
+    EXPECT_FALSE(m_sut->setReportDecodeErrors());
+}
+
+TEST_F(GstGenericPlayerPrivateTest, shouldFailToSetReportDecodeErrorsIfPropertyDoesntExist)
+{
+    expectGetDecoder(m_element);
+
+    expectPropertyDoesntExist(m_glibWrapperMock, m_gstWrapperMock, m_realElement, kReportDecodeErrorsStr);
+    EXPECT_CALL(*m_gstWrapperMock, gstObjectUnref(m_realElement)).Times(1);
+    EXPECT_FALSE(m_sut->setReportDecodeErrors());
+}
+
+TEST_F(GstGenericPlayerPrivateTest, shouldSetReportDecodeErrors)
+{
+    expectGetDecoder(m_element);
+
+    expectSetProperty(m_glibWrapperMock, m_gstWrapperMock, m_realElement, kReportDecodeErrorsStr, true);
+    EXPECT_CALL(*m_gstWrapperMock, gstObjectUnref(m_realElement)).Times(1);
+    EXPECT_TRUE(m_sut->setReportDecodeErrors());


These new tests call m_sut->setReportDecodeErrors() with no arguments, but IGstGenericPlayerPrivate::setReportDecodeErrors was added with a bool reportDecodeErrors parameter. As written, this won't compile; pass the expected bool value (and adjust the expectations accordingly).

mhughesacn · 2026-02-11T11:05:40Z

Hi @Koky2701 : Please use the name "Comcast Cable Communications Management, LLC" for the Comcast company name in the 4 files: SetReportDecodeErrorsTest.cpp (3 files) and GetQueuedFramesTest.cpp.
Please append this two-liner to the end of NOTICE as well:

Copyright 2026 Comcast Cable Communications Management, LLC
Licensed under the Apache License, Version 2.0

Thank you.

github-actions · 2026-02-11T12:43:41Z

tests/componenttests/server/tests/mediaPipeline/PipelinePropertyTest.cpp:350:0: style: The function 'setReportDecodeErrorsFailure' is never used. [unusedFunction]
void setReportDecodeErrorsFailure()
^
tests/componenttests/server/tests/mediaPipeline/PipelinePropertyTest.cpp:359:0: style: The function 'getQueuedFramesFailure' is never used. [unusedFunction]
void getQueuedFramesFailure()
^
nofile:0:0: information: Active checkers: 161/592 (use --checkers-report= to see details) [checkersReport]

rdkcmf-jenkins · 2026-02-11T12:44:27Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 4 files pending identification.

Protex Server Path: /home/blackduck/github/rialto/438/rdkcentral/rialto
Commit: b3811c1

Report detail: gist'

github-actions · 2026-02-11T13:01:18Z

tests/componenttests/server/tests/mediaPipeline/PipelinePropertyTest.cpp:350:0: style: The function 'setReportDecodeErrorsFailure' is never used. [unusedFunction]
void setReportDecodeErrorsFailure()
^
tests/componenttests/server/tests/mediaPipeline/PipelinePropertyTest.cpp:359:0: style: The function 'getQueuedFramesFailure' is never used. [unusedFunction]
void getQueuedFramesFailure()
^
nofile:0:0: information: Active checkers: 161/592 (use --checkers-report= to see details) [checkersReport]

rdkcmf-jenkins · 2026-02-11T13:02:13Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 4 files pending identification.

Protex Server Path: /home/blackduck/github/rialto/438/rdkcentral/rialto
Commit: 40ff73a

Report detail: gist'

Copilot

Pull request overview

Copilot reviewed 64 out of 64 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-20T15:24:40Z

tests/unittests/media/server/service/mediaPipelineService/MediaPipelineServiceTestsFixture.cpp

+void MediaPipelineServiceTests::mediaPipelineWillGetQueuedFrames()
+{
+    EXPECT_CALL(m_mediaPipelineMock, getQueuedFrames(_, _)).WillOnce(DoAll(SetArgReferee<1>(123), Return(true)));
+}
+


This expectation hard-codes 123 when a kQueuedFrames constant already exists in this fixture. Using the constant avoids duplication and keeps the test aligned if the value changes.

Copilot · 2026-02-20T15:24:40Z

media/server/ipc/source/MediaPipelineModuleService.cpp

+                                                 ::google::protobuf::Closure *done)
+{
+    RIALTO_SERVER_LOG_DEBUG("entry:");
+    uint32_t queuedFramesNumber;


queuedFramesNumber is declared without an initial value. Even though it’s only used on the success path, initializing it (e.g., to 0) makes the code more robust against accidental misuse and can avoid static-analysis warnings about potentially uninitialized output values.

Suggested change

uint32_t queuedFramesNumber;

uint32_t queuedFramesNumber{0};

Copilot · 2026-02-20T15:24:41Z

tests/unittests/media/client/main/mediaPipeline/GetQueuedFramesTest.cpp

+        .WillOnce(DoAll(SetArgReferee<1>(123), Return(true)));
+    uint32_t QueuedFrames;
+    EXPECT_TRUE(m_mediaPipeline->getQueuedFrames(m_kSourceId, QueuedFrames));
+    EXPECT_EQ(kExpectedQueuedFrames, QueuedFrames);


Local variable QueuedFrames uses UpperCamelCase, which is inconsistent with surrounding lowerCamelCase variable naming in tests (e.g., queuedFrames). Renaming improves readability and consistency.

Summary: Pending was not used. Type: Fix Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

Copilot

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-20T16:20:07Z

media/public/include/IMediaPipeline.h

+     * This method is synchronous, it gets the queued frames property
+     *
+     * @param[in] sourceId  : The source id. Value should be set to the MediaSource.id returned after attachSource()
+     * @param[out] queuedFrames : Get queued frames on the decoder


The documentation says "Get queued frames on the decoder" but this should be more clear about the parameter direction. It should read "The number of queued frames on the decoder" or "Returns the number of queued frames on the decoder" to match the style of similar getter documentation in this interface.

Suggested change

* @param[out] queuedFrames : Get queued frames on the decoder

* @param[out] queuedFrames : The number of queued frames on the decoder

Copilot · 2026-02-20T16:20:08Z

media/client/ipc/interface/IMediaPipelineIpc.h

+     * This method is synchronous, it gets the queued frames property
+     *
+     * @param[in] sourceId  : The source id. Value should be set to the MediaSource.id returned after attachSource()
+     * @param[out] queuedFrames : Get queued frames on the decoder


The documentation says "Get queued frames on the decoder" but should be more descriptive for an output parameter. It should read "The number of queued frames on the decoder" or "Returns the number of queued frames on the decoder" to match the documentation style for similar getter methods.

Suggested change

* @param[out] queuedFrames : Get queued frames on the decoder

* @param[out] queuedFrames : The number of queued frames on the decoder

Copilot · 2026-02-20T16:20:08Z

media/server/gstplayer/source/tasks/generic/SetupElement.cpp

@@ -333,7 +337,6 @@ void SetupElement::execute() const
    {
        m_gstWrapper->gstBaseParseSetPtsInterpolation(GST_BASE_PARSE(m_element), FALSE);
    }


This deletion of a blank line appears to be an unintentional change unrelated to the feature being added. The blank line separation between the conditional block and the gstObjectUnref call improves code readability and should be retained.

Suggested change

}

}

Copilot · 2026-02-20T16:20:08Z

media/server/gstplayer/include/tasks/generic/SetReportDecodeErrors.h

+ * If not stated otherwise in this file or this component's LICENSE file the
+ * following copyright and licenses apply:
+ *
+ * Copyright 2026 Sky UK


The copyright year is inconsistent across the new files. This file has "Copyright 2026 Sky UK" while other new files in this PR have "Copyright 2025 Sky UK" and "Copyright 2024 Sky UK". Since the PR is being created in early 2025 (based on context), the copyright year should be "2025" to match other recently created files in the PR.

Suggested change

* Copyright 2026 Sky UK

* Copyright 2025 Sky UK

Copilot · 2026-02-20T16:20:09Z

proto/mediapipelinemodule.proto

+message ReportDecodeErrorsResponse {
+}
+
+


There is an extra blank line here that is inconsistent with the formatting of other message definitions in this file. Message pairs (Request/Response) should have only one blank line between them and the next message, as seen with other definitions like SetImmediateOutputResponse and getImmediateOutput.

Suggested change

Copilot · 2026-02-20T16:20:09Z

tests/unittests/media/client/ipc/mediaPipelineIpc/GetQueuedFramesTest.cpp

+ * If not stated otherwise in this file or this component's LICENSE file the
+ * following copyright and licenses apply:
+ *
+ * Copyright 2024 Sky UK


The copyright year "2024" is inconsistent with other new test files in this PR that use "2025". Since this is a new file being added in early 2025, it should have "Copyright 2025 Sky UK" to match files like SetReportDecodeErrorsTest.cpp and GetQueuedFramesTest.cpp in the same directory.

github-actions · 2026-02-20T16:25:28Z

Coverage statistics of your commit:
Congratulations, your commit improved lines coverage from: 85.2% to 85.3%
Functions coverage stays unchanged and is: 92.5%

Copilot

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T09:07:15Z

tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp

+TEST_F(GstGenericPlayerTest, shouldFailToGetQueuedFramesInPlayingStateIfMediaTypeWrong)
+{
+    setPipelineState(GST_STATE_PLAYING);
+
+    uint32_t queuedFrames;
+    EXPECT_FALSE(m_sut->getQueuedFrames(MediaSourceType::UNKNOWN, queuedFrames));
+}


This test calls m_sut->getQueuedFrames(MediaSourceType::UNKNOWN, queuedFrames), but GstGenericPlayer/IGstGenericPlayer currently expose getQueuedFrames(uint32_t&) only. This is a signature mismatch that will fail compilation. Update the test to match the final API (and add media-type validation coverage if the API is updated to accept MediaSourceType).

Copilot · 2026-02-23T09:07:15Z

media/server/gstplayer/source/tasks/generic/SetReportDecodeErrors.cpp

+void SetReportDecodeErrors::execute() const
+{
+    RIALTO_SERVER_LOG_DEBUG("Executing SetReportDecodeErrors for %s source", common::convertMediaSourceType(m_type));
+


The new SetReportDecodeErrors task unconditionally writes pendingReportDecodeErrorsForVideo regardless of m_type. This differs from SetImmediateOutput (which rejects non-video types) and could accidentally apply the setting when called for AUDIO/UNKNOWN. Gate this logic on m_type == MediaSourceType::VIDEO (and log/return false for other types) to keep per-source behavior correct.

Suggested change

if (MediaSourceType::VIDEO != m_type)

{

RIALTO_SERVER_LOG_WARN("SetReportDecodeErrors is only valid for VIDEO sources (got %s)",

common::convertMediaSourceType(m_type));

return;

}

Copilot · 2026-02-23T09:07:15Z

tests/unittests/media/server/main/mediaPipeline/MiscellaneousFunctionsTest.cpp

+    int videoSourceId = attachSource(firebolt::rialto::MediaSourceType::VIDEO, "video/h264");
+    mainThreadWillEnqueueTaskAndWait();
+    EXPECT_CALL(*m_gstPlayerMock, getQueuedFrames(_, _)).WillOnce(Return(false));
+    uint32_t queuedFrames;
+    EXPECT_FALSE(m_mediaPipeline->getQueuedFrames(videoSourceId, queuedFrames));
+}


The expectations for m_gstPlayerMock->getQueuedFrames(_, _) assume a 2-argument API, but IGstGenericPlayer::getQueuedFrames is currently declared with a single uint32_t& out-param. This will fail to compile once the mock/interface are aligned. Update this test to match the final getQueuedFrames signature (and verify the correct media source is used if a MediaSourceType parameter is added).

Copilot · 2026-02-23T09:07:16Z

media/server/service/source/MediaPipelineService.cpp

+    if (mediaPipelineIter == m_mediaPipelines.end())
+    {
+        RIALTO_SERVER_LOG_ERROR("Session with id: %d does not exists", sessionId);
+        return false;


Log message grammar: "does not exists" should be "does not exist".

Copilot · 2026-02-23T09:07:16Z

media/server/service/source/MediaPipelineService.cpp

+    {
+        RIALTO_SERVER_LOG_ERROR("Session with id: %d does not exists", sessionId);
+        return false;
+    }


Log message grammar: "does not exists" should be "does not exist".

Copilot · 2026-02-23T09:07:16Z

media/server/main/include/MediaPipelineServerInternal.h

+     * @brief Sets the "Report Decode Errors" property for this source.
+     *
+     * This method is asynchronous
+     *
+     * @param[in] sourceId : The source id. Value should be set to the MediaSource.id returned after attachSource()
+     * @param[in] reportDecodeErrors : The desired Set Report Decode Errors mode on the sink
+     *
+     * @retval true on success.
+     */
+    bool setReportDecodeErrorsInternal(int32_t sourceId, bool reportDecodeErrors);


The Doxygen for setReportDecodeErrorsInternal says the setting is applied "on the sink", but the implementation applies report_decode_errors on the decoder element. Please update the comment to reflect where the property is set (decoder) to avoid misleading API documentation.

Copilot · 2026-02-23T09:07:16Z

tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerMock.h

    MOCK_METHOD(bool, getImmediateOutput, (const MediaSourceType &mediaSourceType, bool &immediateOutput), (override));
+    MOCK_METHOD(bool, setReportDecodeErrors, (const MediaSourceType &mediaSourceType, bool reportDecodeErrors),
+                (override));
+    MOCK_METHOD(bool, getQueuedFrames, (const MediaSourceType &mediaSourceType, uint32_t &queuedFrames), (override));
    MOCK_METHOD(bool, getStats,


getQueuedFrames mock signature includes a MediaSourceType argument, but IGstGenericPlayer::getQueuedFrames is declared as getQueuedFrames(uint32_t&). This will not compile because the method marked override doesn't match the interface. Update the mock (and any expectations) to match the interface, or update the interface/implementation to accept MediaSourceType consistently.

Copilot · 2026-02-23T09:07:17Z

media/server/main/source/MediaPipelineServerInternal.cpp

+        RIALTO_SERVER_LOG_ERROR("Failed - Source not found");
+        return false;
+    }
+    return m_gstPlayer->getQueuedFrames(queuedFrames);


getQueuedFramesInternal validates the sourceId but then ignores the resolved media source type (sourceIter->first) when calling the gst player. As written, it will always query the player's default (currently VIDEO) queued frames even if the sourceId refers to a different source, and it also won't compile if the gst player API expects a media type. Use the resolved media type when calling into m_gstPlayer, or explicitly reject non-video sources here.

Suggested change

return m_gstPlayer->getQueuedFrames(queuedFrames);

return m_gstPlayer->getQueuedFrames(sourceIter->first, queuedFrames);

Copilot · 2026-02-23T09:07:17Z

...tests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTestsFixture.cpp

+void MediaPipelineModuleServiceTests::sendGetQueuedFramesRequestAndReceiveFail()
+{
+    firebolt::rialto::GetQueuedFramesRequest request;
+    firebolt::rialto::GetQueuedFramesResponse response;
+
+    request.set_session_id(kHardcodedSessionId);
+
+    m_service->getQueuedFrames(m_controllerMock.get(), &request, &response, m_closureMock.get());
+}


GetQueuedFramesRequest includes a source_id field, but this test request only sets session_id, so it doesn't validate correct per-source routing. Set source_id and update the service mock expectation to match, otherwise a regression where source_id is ignored would still pass.

Copilot · 2026-02-23T09:07:17Z

media/server/gstplayer/interface/IGstGenericPlayer.h

+     * @param[out] queuedFrames : Get queued frames mode on the decoder
+     *
+     * @retval true on success.
+     */
+    virtual bool getQueuedFrames(uint32_t &queuedFrames) = 0;


IGstGenericPlayer::getQueuedFrames is declared with only an output parameter, but other updated code in this PR (e.g., mocks/tests and MediaPipelineServerInternal) treats it as a per-source operation that needs a MediaSourceType/source context. This inconsistency will cause override/signature mismatches and makes it unclear which source the queued frames belong to. Align the API across interface, implementation, and callers (either add a MediaSourceType parameter like other per-source getters, or update mocks/tests/callers to the 1-arg form and explicitly validate that only VIDEO sources are supported).

Suggested change

* @param[out] queuedFrames : Get queued frames mode on the decoder

*

* @retval true on success.

*/

virtual bool getQueuedFrames(uint32_t &queuedFrames) = 0;

* @param[in] mediaSourceType : The media source type

* @param[out] queuedFrames : Get queued frames mode on the decoder

*

* @retval true on success.

*/

virtual bool getQueuedFrames(const MediaSourceType &mediaSourceType, uint32_t &queuedFrames) = 0;

Copilot

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T09:21:54Z

media/server/main/include/MediaPipelineServerInternal.h

+     * This method is asynchronous
+     *
+     * @param[in] sourceId : The source id. Value should be set to the MediaSource.id returned after attachSource()
+     * @param[in] queuedFrames : Number of queued frames
+     *
+     * @retval true on success.
+     */
+    bool getQueuedFramesInternal(int32_t sourceId, uint32_t &queuedFrames);


Doxygen annotation marks queuedFrames as [in], but it is an output parameter (uint32_t &queuedFrames). Update the comment to [out] to avoid misleading API documentation.

Copilot · 2026-02-23T09:21:54Z

media/server/gstplayer/source/tasks/generic/SetupElement.cpp

+            if (m_context.pendingReportDecodeErrorsForVideo.has_value())
+            {
+                m_player.setReportDecodeErrors();
+            }
        }
    }



pendingReportDecodeErrorsForVideo is only applied here when a subtitle sink exists and the video decoder handle is being set. If subtitles are not used, the pending report_decode_errors value may never be applied to the decoder. Consider applying setReportDecodeErrors() whenever the video decoder element is discovered (independent of subtitleSink/isVideoHandleSet).

Suggested change

if (m_context.pendingReportDecodeErrorsForVideo.has_value())

{

m_player.setReportDecodeErrors();

}

}

}

}

}

if (isVideoDecoder(*m_gstWrapper, m_element) && m_context.pendingReportDecodeErrorsForVideo.has_value())

{

m_player.setReportDecodeErrors();

}

Copilot · 2026-02-23T09:21:54Z

media/server/gstplayer/source/GstGenericPlayer.cpp

+    GstElement *decoder = getDecoder(MediaSourceType::VIDEO);
+    if (decoder)
+    {
+        RIALTO_SERVER_LOG_DEBUG("Set report decode errors to %s", reportDecodeErrors ? "TRUE" : "FALSE");
+
+        if (m_glibWrapper->gObjectClassFindProperty(G_OBJECT_GET_CLASS(decoder), "report_decode_errors"))
+        {
+            gboolean reportDecodeErrorsGboolean{reportDecodeErrors ? TRUE : FALSE};
+            m_glibWrapper->gObjectSet(decoder, "report_decode_errors", reportDecodeErrorsGboolean, nullptr);


PR description mentions report_decode_errors missing from RialtoMSEVideoSink, but the implementation sets/queries report_decode_errors on the VIDEO decoder element (getDecoder(MediaSourceType::VIDEO)). Please confirm whether the property is expected on the sink or decoder and adjust the element lookup accordingly to match the intended GStreamer element.

Copilot · 2026-02-23T09:21:55Z

media/server/main/source/MediaPipelineServerInternal.cpp

+        RIALTO_SERVER_LOG_ERROR("Failed - Source not found");
+        return false;
+    }
+


setReportDecodeErrorsInternal forwards the request based only on sourceId presence. Since the underlying implementation currently targets the video decoder, an AUDIO sourceId would still end up toggling the video decoder setting. Add a check that the matched source type is MediaSourceType::VIDEO (or otherwise handle per-type state) and fail for unsupported types.

Suggested change

// setReportDecodeErrors currently targets the video decoder; reject unsupported source types

if (sourceIter->first != MediaSourceType::VIDEO)

{

RIALTO_SERVER_LOG_ERROR("Failed - setReportDecodeErrors is supported only for video sources");

return false;

}

Summary: Redundant argument. Type: Fix Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

Copilot

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T09:43:17Z

tests/componenttests/server/tests/mediaPipeline/PipelinePropertyTest.cpp

    stop();

-    // Step 19: Destroy media session
+    // Step 18: Destroy media session


The comment on line 600 says "Step 18: Destroy media session" but was previously "Step 19: Destroy media session". This change appears to be incorrect since no steps were actually removed from the test - new functionality was added but the existing steps remain the same. The comment should remain as "Step 19" or the test should be updated to include the new property testing steps.

github-actions · 2026-02-23T09:49:28Z

Coverage statistics of your commit:
Congratulations, your commit improved lines coverage from: 85.1% to 85.2%
Functions coverage stays unchanged and is: 92.5%

github-actions · 2026-02-23T10:42:16Z

Coverage statistics of your commit:
Congratulations, your commit improved lines coverage from: 85.1% to 85.2%
Functions coverage stays unchanged and is: 92.5%

Koky2701 added 4 commits January 28, 2026 12:20

Added report decode errors rpc in proto

d8f1b1c

Summary: 'report_decode_errors' property missing from RialtoMSEVideoSink Type: Feature Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

Added report decode errors feature. Added build fail fix in mediapipe…

c0ec6dc

…linemodule.proto Summary: 'report_decode_errors' property missing from RialtoMSEVideoSink Type: Feature Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

Added queued frames rpc in proto

0c57f2f

Summary: 'report_decode_errors' property missing from RialtoMSEVideoSink Type: Feature Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

Added build error fix for report decode error feature

3c0c5ae

Summary: 'report_decode_errors' property missing from RialtoMSEVideoSink Type: Feature Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

Copilot AI review requested due to automatic review settings January 28, 2026 11:22

Copilot started reviewing on behalf of Koky2701 January 28, 2026 11:22 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 3, 2026 14:17

Koky2701 force-pushed the feature/RDKEMW-12692 branch from 418abb7 to 50520a8 Compare February 3, 2026 14:17

Copilot started reviewing on behalf of Koky2701 February 3, 2026 14:17 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

Added queued frames feature. Fixed comment errors.

d704fc2

Summary: 'queued_buffers' property missing from RialtoMSEVideoSink Type: Feature Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

Koky2701 force-pushed the feature/RDKEMW-12692 branch from 50520a8 to d704fc2 Compare February 3, 2026 16:12

Copilot AI review requested due to automatic review settings February 10, 2026 17:07

Copilot started reviewing on behalf of Koky2701 February 10, 2026 17:07 View session

Copilot AI reviewed Feb 10, 2026

View reviewed changes

Koky2701 force-pushed the feature/RDKEMW-12692 branch from 1fe58a5 to b3811c1 Compare February 11, 2026 12:43

Copilot AI review requested due to automatic review settings February 11, 2026 13:00

Koky2701 force-pushed the feature/RDKEMW-12692 branch from b3811c1 to 40ff73a Compare February 11, 2026 13:00

Copilot started reviewing on behalf of Koky2701 February 11, 2026 13:01 View session

Koky2701 force-pushed the feature/RDKEMW-12692 branch from a33e411 to 9daca68 Compare February 20, 2026 14:47

Copilot AI review requested due to automatic review settings February 20, 2026 15:20

Koky2701 force-pushed the feature/RDKEMW-12692 branch from 9daca68 to 5be2aba Compare February 20, 2026 15:20

Copilot started reviewing on behalf of Koky2701 February 20, 2026 15:20 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

Koky2701 force-pushed the feature/RDKEMW-12692 branch from 5be2aba to 19f5878 Compare February 20, 2026 15:41

Added fix for pending report_decode_errors call.

fb0f618

Summary: Pending was not used. Type: Fix Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

Copilot AI review requested due to automatic review settings February 20, 2026 16:15

Koky2701 force-pushed the feature/RDKEMW-12692 branch from 19f5878 to fb0f618 Compare February 20, 2026 16:15

Copilot started reviewing on behalf of Koky2701 February 20, 2026 16:15 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 23, 2026 08:59

Koky2701 force-pushed the feature/RDKEMW-12692 branch from a0c4f24 to eea15c5 Compare February 23, 2026 08:59

Copilot started reviewing on behalf of Koky2701 February 23, 2026 08:59 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

Koky2701 force-pushed the feature/RDKEMW-12692 branch from eea15c5 to 31c31c5 Compare February 23, 2026 09:11

Copilot AI review requested due to automatic review settings February 23, 2026 09:15

Koky2701 force-pushed the feature/RDKEMW-12692 branch from 31c31c5 to c867d8e Compare February 23, 2026 09:15

Copilot started reviewing on behalf of Koky2701 February 23, 2026 09:16 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

Koky2701 force-pushed the feature/RDKEMW-12692 branch from c867d8e to fec5278 Compare February 23, 2026 09:29

Added fix for redundant MediaSourceType in getQueuedFrames function.

14d4efd

Summary: Redundant argument. Type: Fix Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692

Copilot AI review requested due to automatic review settings February 23, 2026 09:38

Koky2701 force-pushed the feature/RDKEMW-12692 branch from fec5278 to 14d4efd Compare February 23, 2026 09:38

Copilot started reviewing on behalf of Koky2701 February 23, 2026 09:39 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

Merge branch 'master' into feature/RDKEMW-12692

7e5a8f6

	* @param[in] immediateOutput : Set Report Decode Errors mode on the sink
	* @param[in] reportDecodeErrors : Set Report Decode Errors mode on the sink

		#ifndef FIREBOLT_RIALTO_SERVER_TASKS_GENERIC_SET_REPORT_DECODE_ERROR_H_
		#define FIREBOLT_RIALTO_SERVER_TASKS_GENERIC_SET_REPORT_DECODE_ERROR_H_

	* @param[in] reportDecodeErrors : The desired immediate output mode on the sink
	* @param[in] reportDecodeErrors : The desired report decode errors mode on the sink

	RIALTO_SERVER_LOG_DEBUG("Pending an report_decode_errors, decoder is NULL");
	RIALTO_SERVER_LOG_DEBUG("Pending a report_decode_errors, decoder is NULL");

	* @param[in] sourceId : The source id. Value should be get to the MediaSource.id returned after attachSource()
	* @param[in] sourceId : The source id. Value should be set to the MediaSource.id returned after attachSource()

	RIALTO_CLIENT_LOG_ERROR("failed to get immediate-output due to '%s'", ipcController->ErrorText().c_str());
	RIALTO_CLIENT_LOG_ERROR("failed to get queued frames due to '%s'", ipcController->ErrorText().c_str());

		bool setReportDecodeErrors(int32_t sourceId, bool reportDecodeErrors) override;

		bool getQueuedFrames(int32_t sourceId, uint32_t &queuedFrames) override;

		constexpr bool kReportDecodeErrorsVal{false};
		constexpr bool kQueuedFramesVal{123};

	* @param[out] queuedFrames : Get queued frames on the decoder
	* @param[out] queuedFrames : The number of queued frames on the decoder

+    if (MediaSourceType::VIDEO != m_type)
+    {
+        RIALTO_SERVER_LOG_WARN("SetReportDecodeErrors is only valid for VIDEO sources (got %s)",
+                               common::convertMediaSourceType(m_type));
+        return;
+    }

	return m_gstPlayer->getQueuedFrames(queuedFrames);
	return m_gstPlayer->getQueuedFrames(sourceIter->first, queuedFrames);

+    // setReportDecodeErrors currently targets the video decoder; reject unsupported source types
+    if (sourceIter->first != MediaSourceType::VIDEO)
+    {
+        RIALTO_SERVER_LOG_ERROR("Failed - setReportDecodeErrors is supported only for video sources");
+        return false;
+    }

Comments

Conversation

Koky2701 commented Jan 28, 2026

Uh oh!

github-actions bot commented Jan 28, 2026

Uh oh!

github-actions bot commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Koky2701 commented Feb 10, 2026

Uh oh!

github-actions bot commented Jan 28, 2026 •

edited

Loading